Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use xV_FROM_REF() macros in place of casting result of SvRV() #22417

Merged
merged 3 commits into from
Jul 22, 2024

Conversation

leonerd
Copy link
Contributor

@leonerd leonerd commented Jul 20, 2024

This results in shorter neater code, and additional debugging assertions that the dereferenced SVs really are the requested type when built under -DDEBUGGING.

When I added the xV_FROM_REF() macros, I searched for (TYPE)SvRV style cast expressions, but forgot to additionally look for MUTABLE_xV() calls.

There are additionally two spots in pp_hot.c that cannot be modified, because despite casting the result to a CV pointer, the SV isn't actually a CV. I've added a comment on these lines as to why they're not altered.

@khwilliamson
Copy link
Contributor

khwilliamson commented Jul 21, 2024

But, I noticed that the macro these call use a variable named _ref. Names with a leading underscore are reserved for the C implementation itself. ref_ would be legal for us to use. The consequences of the name in use are unlikely to bite us, so this is for future reference for anyone reading this

@mauke
Copy link
Contributor

mauke commented Jul 21, 2024

Names with a leading underscore are reserved for the C implementation itself

... but only at file scope. _ref is fine as a local variable.

@khwilliamson
Copy link
Contributor

Names with a leading underscore are reserved for the C implementation itself

... but only at file scope. _ref is fine as a local variable.

If I'm wrong about this, please review "Choosing legal symbol names" in perlhacktips

@mauke
Copy link
Contributor

mauke commented Jul 22, 2024

If I'm wrong about this, please review "Choosing legal symbol names" in perlhacktips

https://perldoc.perl.org/5.41.2/perlhacktips#Choosing-legal-symbol-names agrees with me:

C reserves for its implementation any symbol whose name begins with an underscore followed immediately by either an uppercase letter [A-Z] or another underscore. C++ further reserves any symbol containing two consecutive underscores, and further reserves in the global name space any symbol beginning with an underscore, not just ones followed by a capital.

@leonerd
Copy link
Contributor Author

leonerd commented Jul 22, 2024

I notice other macros use ref_ in this situation anyway, so I'll fix this one accordingly.

@leonerd leonerd force-pushed the use-xv-from-ref branch 2 times, most recently from 7fa858d to df05975 Compare July 22, 2024 15:06
leonerd added 3 commits July 22, 2024 16:52
This results in shorter neater code, and additional debugging assertions
that the dereferenced SVs really are the requested type when built under
`-DDEBUGGING`.

When I added the xV_FROM_REF() macros, I searched for `(TYPE)SvRV` style
cast expressions, but forgot to additionally look for `MUTABLE_xV()`
calls.

There are additionally two spots in pp_hot.c that cannot be modified,
because despite casting the result to a CV pointer, the SV isn't
actually a CV. I've added a comment on these lines as to why they're not
altered.
@leonerd leonerd merged commit 352ca4b into Perl:blead Jul 22, 2024
33 checks passed
@leonerd leonerd deleted the use-xv-from-ref branch July 22, 2024 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants